Conversation
Both validation/auth failures and sandbox creation errors were being returned as JSON responses without any server-side logging, making it impossible to diagnose issues from Vercel function logs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded non-functional runtime logging: input params logged during sandbox creation, validation/exception responses logged in the sandbox post handler, and masked API-key, key hash, and matched-key count logged in API key lookup. No exported signatures or control-flow outcomes were changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/sandbox/createSandbox.ts (1)
40-40: Consider potential sensitive data in logged params.The
CreateSandboxParamsobject could contain environment variables or other sensitive configuration in thesourceproperty. While this logging is useful for debugging, ensure that callers don't inadvertently pass secrets in the params that would then appear in Vercel function logs.If this is temporary debugging code intended to be removed after diagnosing Sidney's issue, consider adding a
// TODO: remove after debuggingcomment to signal intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/createSandbox.ts` at line 40, The console.log in createSandbox that prints JSON.stringify(params) may leak secrets (notably params.source); replace it with safer logging: either remove the log, redact sensitive keys (e.g., mask params.source, env, secrets) before logging, or wrap it with a temporary TODO comment indicating it's for debugging and must be removed; update the log call in the createSandbox function to only emit non-sensitive fields or a redacted summary and add a "// TODO: remove after debugging" comment if keeping it temporarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/sandbox/createSandbox.ts`:
- Line 40: The console.log in createSandbox that prints JSON.stringify(params)
may leak secrets (notably params.source); replace it with safer logging: either
remove the log, redact sensitive keys (e.g., mask params.source, env, secrets)
before logging, or wrap it with a temporary TODO comment indicating it's for
debugging and must be removed; update the log call in the createSandbox function
to only emit non-sensitive fields or a redacted summary and add a "// TODO:
remove after debugging" comment if keeping it temporarily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a5ac450-99cf-4eba-bae4-85b1fa776a26
📒 Files selected for processing (2)
lib/sandbox/createSandbox.tslib/sandbox/createSandboxPostHandler.ts
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: This PR only adds diagnostic logging to assist with debugging, which has no impact on business logic or system behavior.
Architecture diagram
sequenceDiagram
participant Client
participant Handler as POST Handler
participant Validator as Body Validator
participant SDK as Sandbox SDK
participant Logs as Vercel Logs
Client->>Handler: POST /api/sandboxes
Handler->>Validator: validateSandboxBody(request)
alt Validation or Auth Fails
Validator-->>Handler: NextResponse (Error)
Handler->>Logs: NEW: console.error([POST /api/sandboxes] Validation/auth failed)
Handler-->>Client: 4xx/401 Response
else Validation Success
Validator-->>Handler: Validated Data
Handler->>SDK: createSandbox(params)
Note over SDK,Logs: Logging inputs for debugging
SDK->>Logs: NEW: console.log([createSandbox] Creating sandbox with params)
SDK->>SDK: Sandbox.create(params)
alt Creation Success
SDK-->>Handler: Sandbox Object
Handler-->>Client: 200 OK
else Exception Caught
SDK-->>Handler: Throw Error
Handler->>Logs: NEW: console.error([POST /api/sandboxes] Error creating sandbox)
Handler-->>Client: 500 Error Response
end
end
Logs key prefix, computed hash, and match count to help diagnose why prod API key lookups return no results. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/auth/getApiKeyAccountId.ts (1)
14-79: Function exceeds 50-line guideline (currently ~66 lines).Per the coding guidelines, domain functions in
lib/**/*.tsshould stay under 50 lines. While this predates the current PR, the added logging is a good opportunity to consider extracting the error-response logic into a small helper (e.g.,buildAuthErrorResponse(message, status)) to reduce repetition and line count.Not blocking, but worth considering for maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth/getApiKeyAccountId.ts` around lines 14 - 79, getApiKeyAccountId is over the 50-line limit due to repeated error-response boilerplate; extract a small helper (e.g., buildAuthErrorResponse(message: string, status: number): NextResponse) and replace the three JSON error returns inside getApiKeyAccountId (the missing x-api-key branch, the Unauthorized branch, and the catch/500 branch that also handles selectAccountApiKeys null) with calls to this helper, keeping existing use of getCorsHeaders(), and leave hashing/logging and the selectAccountApiKeys call intact to reduce lines and duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/auth/getApiKeyAccountId.ts`:
- Around line 32-35: In getApiKeyAccountId, stop logging sensitive portions of
the API key and hash: replace the apiKey.slice(0,12) log with a masked form
showing only the last 4 characters (e.g., "..." + apiKey.slice(-4)) and log only
a truncated hash prefix (e.g., keyHash.slice(0,8) + "..."); gate all diagnostic
console.log calls (the ones referencing apiKey and keyHash and the matched keys
count around selectAccountApiKeys) behind an environment flag like DEBUG_AUTH so
they only run when enabled, and add a TODO comment "// TODO: Remove diagnostic
logging after `#411` investigation" next to the gated logs for cleanup tracking.
---
Nitpick comments:
In `@lib/auth/getApiKeyAccountId.ts`:
- Around line 14-79: getApiKeyAccountId is over the 50-line limit due to
repeated error-response boilerplate; extract a small helper (e.g.,
buildAuthErrorResponse(message: string, status: number): NextResponse) and
replace the three JSON error returns inside getApiKeyAccountId (the missing
x-api-key branch, the Unauthorized branch, and the catch/500 branch that also
handles selectAccountApiKeys null) with calls to this helper, keeping existing
use of getCorsHeaders(), and leave hashing/logging and the selectAccountApiKeys
call intact to reduce lines and duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cfaa5d7e-6267-474a-afeb-21944c81982d
📒 Files selected for processing (1)
lib/auth/getApiKeyAccountId.ts
| console.log("[getApiKeyAccountId] key prefix:", apiKey.slice(0, 12) + "..."); | ||
| console.log("[getApiKeyAccountId] hash:", keyHash); | ||
| const apiKeys = await selectAccountApiKeys({ keyHash }); | ||
| console.log("[getApiKeyAccountId] matched keys:", apiKeys?.length ?? "null"); |
There was a problem hiding this comment.
Security concern: Logging 12 characters of an API key exposes too much of the secret.
Logging apiKey.slice(0, 12) reveals a significant portion of the credential. Depending on key format and entropy distribution, this could meaningfully narrow down the key space or violate compliance policies around secrets in logs (Vercel logs may be accessible to broader teams).
Additionally, logging the full keyHash enables cross-log correlation and could theoretically aid offline attacks if the key space is constrained.
Recommendations:
- Mask more aggressively—show only the last 4 characters:
"..." + apiKey.slice(-4) - Consider gating this diagnostic logging behind an environment variable (e.g.,
DEBUG_AUTH=true) so it doesn't persist in production once the issue is resolved. - Add a
// TODO: Remove diagnostic logging afterfix: add logging to POST /api/sandboxes #411investigationcomment to ensure cleanup.
🔒 Proposed safer masking
- console.log("[getApiKeyAccountId] key prefix:", apiKey.slice(0, 12) + "...");
- console.log("[getApiKeyAccountId] hash:", keyHash);
+ // TODO: Remove diagnostic logging after `#411` investigation
+ console.log("[getApiKeyAccountId] key suffix:", "..." + apiKey.slice(-4));
const apiKeys = await selectAccountApiKeys({ keyHash });
- console.log("[getApiKeyAccountId] matched keys:", apiKeys?.length ?? "null");
+ console.log("[getApiKeyAccountId] matched keys:", apiKeys?.length ?? "null");If you need the hash for correlation, consider logging only a truncated portion:
console.log("[getApiKeyAccountId] hash prefix:", keyHash.slice(0, 8) + "...");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/auth/getApiKeyAccountId.ts` around lines 32 - 35, In getApiKeyAccountId,
stop logging sensitive portions of the API key and hash: replace the
apiKey.slice(0,12) log with a masked form showing only the last 4 characters
(e.g., "..." + apiKey.slice(-4)) and log only a truncated hash prefix (e.g.,
keyHash.slice(0,8) + "..."); gate all diagnostic console.log calls (the ones
referencing apiKey and keyHash and the matched keys count around
selectAccountApiKeys) behind an environment flag like DEBUG_AUTH so they only
run when enabled, and add a TODO comment "// TODO: Remove diagnostic logging
after `#411` investigation" next to the gated logs for cleanup tracking.
Logs snapshot ID resolution, snapshot vs fresh creation failures, and insertAccountSandbox errors to trace the exact failure point. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Superseded by #412 which includes the fix and proper logging. |
Summary
console.errorlogging when validation/auth fails in the POST handler (logs the error body)console.errorlogging when sandbox creation throws an exceptionconsole.logfor sandbox creation params to trace what's being sent toSandbox.createBoth sidney@recoupable.com and sidney+1@recoupable.com are hitting errors on POST /api/sandboxes with no visibility in Vercel function logs. This PR adds the logging needed to diagnose the root cause.
Test plan
[POST /api/sandboxes]log lines🤖 Generated with Claude Code
Summary by cubic
Add server-side logging to POST /api/sandboxes so validation/auth failures and sandbox creation errors show in Vercel logs. Also log sandbox params in
createSandbox, API key diagnostics ingetApiKeyAccountId(prefix, hash, match count), and snapshot lookup, fallback, and insert errors inprocessCreateSandbox.Written for commit 8a08a90. Summary will update on new commits.
Summary by CodeRabbit